Skip to content

Major Cmake / C++ include refactor#191

Open
scal444 wants to merge 4 commits into
NVIDIA-BioNeMo:mainfrom
scal444:cmake_major_include_refactor
Open

Major Cmake / C++ include refactor#191
scal444 wants to merge 4 commits into
NVIDIA-BioNeMo:mainfrom
scal444:cmake_major_include_refactor

Conversation

@scal444
Copy link
Copy Markdown
Collaborator

@scal444 scal444 commented Jun 1, 2026

Removed all of our manual in-tree include CMake code. All includes are now relative to the project root, with most of them under src/.

Added a new formatter checker to CI to avoid backslide.

scal444 added 3 commits April 29, 2026 10:54
Switch every project-local #include "..." to use the path relative to the
repository root, e.g.

  #include "src/forcefields/mmff.h"
  #include "rdkit_extensions/mmff_flattened_builder.h"
  #include "tests/test_utils.h"

Bare neighbor includes ("foo.h"), upward-relative includes ("../foo.h"), and
angle-bracket project-local includes (<foo.h>) are no longer accepted.

Mechanism:
- Add an INTERFACE library nvmolkit_include_root (in the root CMakeLists.txt)
  that exports ${PROJECT_SOURCE_DIR} as its include path. A single
  link_libraries(nvmolkit_include_root) just before the project subdirectories
  applies it to every first-party target.
- Drop the previous per-target target_include_directories(...
  ${CMAKE_CURRENT_SOURCE_DIR}) and ${PROJECT_SOURCE_DIR}/... entries that
  existed only to expose project headers via library include dirs. They are
  now redundant.
- Drop a couple of dead include paths (nvmolkit/utils does not exist;
  smarts_filter no longer needs ${PROJECT_SOURCE_DIR}/src once the include
  rewrite happens).

Add admin/run_include_check.sh + admin/include_check.py to enforce the
policy. Default mode rewrites in place, -d / --check mode reports violations
and exits non-zero. Wired into .github/workflows/lint.yml as a new
include-check job alongside clang-format, cmake-format, and ruff-format.

Configure, build, and ctest pass.
@scal444 scal444 requested a review from evasnow1992 June 1, 2026 15:19
Comment thread admin/include_check.py
)
if rewritten is None:
# Already-correct quoted form, system include, or third-party.
if close_char == ">" and rewritten is None and reason is None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rewritten is None this line seems to be redundant given it's inside the if block from line 189

Copy link
Copy Markdown
Collaborator

@evasnow1992 evasnow1992 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment. Other changes look good to me. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants